-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Engine API: define Open RPC schema #379
Conversation
Were you able to try using -- From a high level view, this PR generally seems good. No way to really know how correct the schema is without some tests, but not sure if you want to add that here. I worked on a tool rpctestgen which makes it pretty easy to update tests for the eth RPC. I think something similar would be useful for the engine api. Although the regeneration is less of an issue because breaking should require a version bump of the method or object. Would be great have sample messages to sanity check the spec, regardless of how they are acquired though. Even just listening in over some EL<>CL interactions and snagging a couple unique method calls would probably suffice for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed a couple things while reviewing manually
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Defines initial OpenRPC schema for Engine API. Draft PR opened up for review, once agreed in general
getPayloadBodies
requests should be added before merging.Folder structure
schemas/
andmethods/
are both put intosrc/engine/openrpc/
to avoid messing Open RPC schema files with method specification files which are main artifacts ofsrc/engine
part of this repo.Schema files follows functional breakdown as it should be easier to read and maintain it this way. I tried to play with functional & version breakdown, like
payload_v1.yaml
,payload_v2.yaml
, and didn't find version breakdown useful at all.Spellchecker
This PR makes spellchecker case insensitive for
.yaml
files, this is done merely to avoid adding words respecting the case to the exception list. If case sensitivity for yamls is important by any mean then we'll have to add those words explicitly.External docs
Tried to play out with
externalDocs
property. Unfortunately, it isn't rendered by OpenRPC playground at all. But I decided to leave this property as it might be useful. Currently it contains full URL to specification of each method.Misc
TBH, I am not satisfied with deduplication framework of JSON schema. I was expecting to define
ExecutionPayloadV2
by giving a command to useExecutionPayloadV1
property set withwithdrawals
field addition. Apparently, it's not allowed. Though, I am not an expert and it might be possible to do, lmk.ToDo